[SDK] Feature: Ox Transaction Envelopes #5598

gregfromstl · 2024-12-03T10:38:37Z

Woah. This is sick.

PR-Codex overview

This PR focuses on updating the thirdweb package to improve transaction serialization by using the ox library, along with refactoring and renaming some files and functions for consistency.

Detailed summary

Renamed coinbaseWebSDK.js to coinbase-web.js across multiple files.
Updated imports for parseTypedData to use parse-typed-data.js.
Introduced SerializableTransaction type in various files.
Added SignableMessage type definition.
Implemented parseTypedData function to handle hex chainId.
Updated sign, signTransaction, and signTypedData functions to use ox library for signing.
Refactored tests to align with new implementations and types.
Implemented error handling in keyless transaction methods.
Enhanced Coinbase wallet functionalities with mobile provider support.

The following files were skipped due to too many changes: packages/thirdweb/src/adapters/ethers5.test.ts, packages/thirdweb/src/utils/hashing/hashTypedData.ts, packages/thirdweb/src/wallets/coinbase/coinbase-web.test.ts, packages/thirdweb/src/transaction/serialize-transaction.ts, packages/thirdweb/src/transaction/serialize-transaction.test.ts, pnpm-lock.yaml

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

changeset-bot · 2024-12-03T10:38:41Z

🦋 Changeset detected

Latest commit: 16e5347

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
thirdweb	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2024-12-03T10:38:41Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs-v2	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 6, 2024 10:30pm
thirdweb_playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 6, 2024 10:30pm
thirdweb-www	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 6, 2024 10:30pm
wallet-ui	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 6, 2024 10:30pm

graphite-app · 2024-12-03T10:38:47Z

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

github-actions · 2024-12-03T10:43:30Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
thirdweb (esm)	41.13 KB (-10.05% 🔽)	823 ms (-10.05% 🔽)	762 ms (-75.76% 🔽)	1.6 s
thirdweb (cjs)	104.48 KB (-1.67% 🔽)	2.1 s (-1.67% 🔽)	2.7 s (-56.19% 🔽)	4.8 s
thirdweb (minimal + tree-shaking)	4.37 KB (-21.76% 🔽)	88 ms (-21.76% 🔽)	136 ms (-73.95% 🔽)	223 ms
thirdweb/chains (tree-shaking)	506 B (0%)	10 ms (0%)	48 ms (+5.42% 🔺)	58 ms
thirdweb/react (minimal + tree-shaking)	16.94 KB (-9.59% 🔽)	339 ms (-9.59% 🔽)	384 ms (-38.56% 🔽)	723 ms

codecov · 2024-12-03T10:45:11Z

Codecov Report

Attention: Patch coverage is 85.77586% with 33 lines in your changes missing coverage. Please review.

Project coverage is 48.16%. Comparing base (406c885) to head (16e5347).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
.../thirdweb/src/transaction/serialize-transaction.ts	88.09%	10 Missing ⚠️
...ages/thirdweb/src/wallets/coinbase/coinbase-web.ts	11.11%	8 Missing ⚠️
.../thirdweb/src/utils/signatures/signature-to-hex.ts	82.60%	4 Missing ⚠️
.../thirdweb/src/utils/any-evm/keyless-transaction.ts	90.00%	3 Missing ⚠️
...s/thirdweb/src/wallets/coinbase/coinbase-wallet.ts	0.00%	2 Missing ⚠️
packages/thirdweb/src/adapters/ethers6.ts	0.00%	1 Missing ⚠️
...s/thirdweb/src/wallets/eip5792/get-calls-status.ts	0.00%	1 Missing ⚠️
...s/thirdweb/src/wallets/eip5792/get-capabilities.ts	0.00%	1 Missing ⚠️
...ackages/thirdweb/src/wallets/eip5792/send-calls.ts	0.00%	1 Missing ⚠️
.../thirdweb/src/wallets/eip5792/show-calls-status.ts	50.00%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5598      +/-   ##
==========================================
+ Coverage   47.86%   48.16%   +0.30%     
==========================================
  Files        1088     1089       +1     
  Lines       56667    56875     +208     
  Branches     4314     4361      +47     
==========================================
+ Hits        27122    27395     +273     
+ Misses      28847    28780      -67     
- Partials      698      700       +2

Flag	Coverage Δ		*Carryforward flag
legacy_packages	`65.68% <ø> (ø)`		Carriedforward from 406c885
packages	`44.02% <85.77%> (+0.39%)`	⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines	Coverage Δ
packages/thirdweb/src/adapters/ethers5.ts	`53.28% <100.00%> (+13.86%)`	⬆️
...b/src/react/web/utils/usePreloadWalletProviders.ts	`43.05% <100.00%> (ø)`
.../transaction/actions/gasless/providers/biconomy.ts	`55.44% <ø> (ø)`
...rc/transaction/actions/gasless/providers/engine.ts	`35.88% <ø> (ø)`
...nsaction/actions/gasless/providers/openzeppelin.ts	`48.41% <ø> (ø)`
...irdweb/src/transaction/actions/sign-transaction.ts	`100.00% <100.00%> (+17.64%)`	⬆️
...transaction/actions/to-serializable-transaction.ts	`76.19% <100.00%> (ø)`
.../src/transaction/actions/zksync/getEip721Domain.ts	`4.68% <ø> (ø)`
packages/thirdweb/src/utils/hashing/hashMessage.ts	`75.00% <100.00%> (-25.00%)`	⬇️
...ckages/thirdweb/src/utils/hashing/hashTypedData.ts	`97.93% <100.00%> (+0.01%)`	⬆️
... and 23 more

jnsdls · 2024-12-04T20:24:24Z

packages/thirdweb/src/utils/signatures/sign-typed-data.test.ts

  );
 });

-test("domain: chainId hex", async () => {


why the removed test?

joaquim-verges · 2024-12-04T21:45:10Z

packages/thirdweb/src/extensions/erc1271/checkContractWalletSignedTypedData.ts

  const result = await isValidSignature({
    contract: options.contract,
-    hash: hashTypedData(options.data),
+    hash: ox__TypedData.hashStruct({


ugh types clashed here?

No I'm just trying to use ox as much as possible in the underlying functions, so we don't need to wrap and/or rewrite internal utils we're not planning to export

i meant the casts underneath this

Oh yes they do

joaquim-verges · 2024-12-04T21:45:44Z

.changeset/little-kids-wash.md

@@ -0,0 +1,5 @@
+---
+"thirdweb": patch


would make this a minor - lots of underlying changes

joaquim-verges · 2024-12-04T21:46:51Z

packages/thirdweb/src/transaction/actions/sign-transaction.ts

@@ -1,11 +1,13 @@
-import type { TransactionSerializable } from "viem";
+import * as ox__Hash from "ox/Hash";


curious why you dont just import { Hash } from 'ox' ?

cjs won't treeshake properly if I do, have to import it this way

really? ugh

joaquim-verges · 2024-12-04T21:48:52Z

packages/thirdweb/src/transaction/serialize-transaction.test.ts


-    const tx = parseTransaction(serialized);
+    const tx = ox__TransactionEnvelopeEip1559.deserialize(
+      serialized as ox__TransactionEnvelopeEip1559.Serialized,


will these cast go away once we migrate everything?

These are just the tests being extra picky because I'm allowing legacy transactions

gregfromstl · 2024-12-06T22:21:31Z

Merge activity

Dec 6, 5:21 PM EST: The merge label 'merge-queue' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Dec 6, 5:21 PM EST: A user added this pull request to the Graphite merge queue.
Dec 6, 5:32 PM EST: A user merged this pull request with the Graphite merge queue.

CNCT-2303 Woah. This is sick.  --- ## PR-Codex overview This PR focuses on updating the `thirdweb` package to enhance transaction serialization by integrating the `ox` library, renaming files, and refactoring code for better consistency in handling typed data and signatures. ### Detailed summary - Replaced instances of `coinbaseWebSDK.js` with `coinbase-web.js`. - Updated the `parseTypedData` function to handle hex `chainId`. - Refactored signature handling in various files to use `ox` library. - Introduced new types and updated existing ones for better type safety. - Added tests for `parseTypedData` functionality. - Adjusted imports across multiple files to reflect new structure. - Enhanced error handling and validation for typed data signatures. > The following files were skipped due to too many changes: `packages/thirdweb/src/adapters/ethers5.test.ts`, `packages/thirdweb/src/utils/hashing/hashTypedData.ts`, `packages/thirdweb/src/wallets/coinbase/coinbase-web.test.ts`, `packages/thirdweb/src/transaction/serialize-transaction.ts`, `packages/thirdweb/src/transaction/serialize-transaction.test.ts`, `pnpm-lock.yaml` > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

gregfromstl requested review from MananTank, edwardysun, jnsdls and joaquim-verges as code owners December 3, 2024 10:38

github-actions bot added packages SDK Involves changes to the thirdweb SDK labels Dec 3, 2024

vercel bot deployed to Preview – wallet-ui December 3, 2024 10:42 View deployment

vercel bot deployed to Preview – thirdweb_playground December 3, 2024 10:42 View deployment

vercel bot had a problem deploying to Preview – thirdweb-www December 3, 2024 10:44 Failure

vercel bot deployed to Preview – docs-v2 December 3, 2024 10:48 View deployment

vercel bot deployed to Preview – wallet-ui December 3, 2024 11:40 View deployment

vercel bot deployed to Preview – thirdweb_playground December 3, 2024 11:41 View deployment

vercel bot deployed to Preview – thirdweb-www December 3, 2024 11:42 View deployment

vercel bot deployed to Preview – docs-v2 December 3, 2024 11:47 View deployment

jnsdls reviewed Dec 4, 2024

View reviewed changes

packages/thirdweb/src/utils/signatures/sign-typed-data.test.ts

);

});

test("domain: chainId hex", async () => {

Copy link

Member

jnsdls Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the removed test?

joaquim-verges reviewed Dec 4, 2024

View reviewed changes

vercel bot deployed to Preview – wallet-ui December 4, 2024 23:45 View deployment

vercel bot deployed to Preview – thirdweb_playground December 4, 2024 23:45 View deployment

vercel bot deployed to Preview – docs-v2 December 4, 2024 23:46 View deployment

vercel bot deployed to Preview – thirdweb-www December 4, 2024 23:47 View deployment

gregfromstl requested review from jnsdls and joaquim-verges December 4, 2024 23:47

vercel bot deployed to Preview – thirdweb_playground December 6, 2024 00:15 View deployment

vercel bot deployed to Preview – docs-v2 December 6, 2024 00:22 View deployment

vercel bot deployed to Preview – thirdweb-www December 6, 2024 00:53 View deployment

vercel bot had a problem deploying to Preview – wallet-ui December 6, 2024 16:57 Failure

vercel bot had a problem deploying to Preview – thirdweb_playground December 6, 2024 16:57 Failure

vercel bot had a problem deploying to Preview – docs-v2 December 6, 2024 16:57 Failure

vercel bot had a problem deploying to Preview – thirdweb-www December 6, 2024 16:57 Failure

vercel bot deployed to Preview – wallet-ui December 6, 2024 19:00 View deployment

vercel bot deployed to Preview – thirdweb_playground December 6, 2024 19:01 View deployment

vercel bot had a problem deploying to Preview – thirdweb-www December 6, 2024 19:02 Failure

vercel bot deployed to Preview – docs-v2 December 6, 2024 19:09 View deployment

vercel bot deployed to Preview – wallet-ui December 6, 2024 19:56 View deployment

vercel bot deployed to Preview – thirdweb_playground December 6, 2024 19:56 View deployment

vercel bot deployed to Preview – thirdweb-www December 6, 2024 19:59 View deployment

vercel bot deployed to Preview – docs-v2 December 6, 2024 20:01 View deployment

gregfromstl added the merge-queue Adds the pull request to Graphite's merge queue. label Dec 6, 2024

gregfromstl force-pushed the 5-dot-ox branch from c22af96 to 16e5347 Compare December 6, 2024 22:22

vercel bot deployed to Preview – wallet-ui December 6, 2024 22:23 View deployment

vercel bot deployed to Preview – thirdweb_playground December 6, 2024 22:23 View deployment

vercel bot deployed to Preview – docs-v2 December 6, 2024 22:30 View deployment

vercel bot deployed to Preview – thirdweb-www December 6, 2024 22:30 View deployment

graphite-app bot merged commit 16e5347 into main Dec 6, 2024
32 checks passed

graphite-app bot deleted the 5-dot-ox branch December 6, 2024 22:32

jnsdls mentioned this pull request Dec 6, 2024

Version Packages #5642

Merged

vercel bot deployed to Production – wallet-ui December 6, 2024 22:34 View deployment

vercel bot deployed to Production – docs-v2 December 6, 2024 22:34 View deployment

vercel bot deployed to Production – thirdweb_playground December 6, 2024 22:35 View deployment

vercel bot deployed to Production – thirdweb-www December 6, 2024 22:44 View deployment

		@@ -1,11 +1,13 @@
		import type { TransactionSerializable } from "viem";
		import * as ox__Hash from "ox/Hash";

[SDK] Feature: Ox Transaction Envelopes #5598

[SDK] Feature: Ox Transaction Envelopes #5598

Uh oh!

Conversation

gregfromstl commented Dec 3, 2024 • edited by pr-codex bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR-Codex overview

Detailed summary

Uh oh!

changeset-bot bot commented Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

vercel bot commented Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

graphite-app bot commented Dec 3, 2024

Your org has enabled the Graphite merge queue for merging into main

Uh oh!

github-actions bot commented Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

codecov bot commented Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gregfromstl commented Dec 6, 2024 • edited by graphite-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gregfromstl commented Dec 3, 2024 •

edited by pr-codex bot

Loading

changeset-bot bot commented Dec 3, 2024 •

edited

Loading

vercel bot commented Dec 3, 2024 •

edited

Loading

github-actions bot commented Dec 3, 2024 •

edited

Loading

codecov bot commented Dec 3, 2024 •

edited

Loading

gregfromstl commented Dec 6, 2024 •

edited by graphite-app bot

Loading